-
Notifications
You must be signed in to change notification settings - Fork 152
fix: regression in non-fast scalar indexing support #760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #760 +/- ##
==========================================
+ Coverage 86.64% 87.15% +0.50%
==========================================
Files 11 12 +1
Lines 1041 1074 +33
==========================================
+ Hits 902 936 +34
+ Misses 139 138 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ext/ForwardDiffGPUArraysCoreExt.jl
Outdated
idxs = collect( | ||
Iterators.drop(ForwardDiff.structural_eachindex(result), offset) | ||
)[1:chunksize] | ||
result[idxs] .= partial_fn.(Ref(dual), 1:chunksize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this not have an inference issue due to losing static information about size? I would think this needs to be ntuple
unless it can prove things about size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would still be type-stable, it would just have dynamism in the function that would slow it down a bit during the broadcast.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here the chunksize is already an Int, so I don't think we will have any benefit of using an ntuple
Noted in #759 (comment), GPU is completely untested in ForwardDiff.jl, so this sets up the buildkite pipeline. I setup the backend and all, and just took a few tests from #760 to seed it. The point of this isn't really to be a comprehensive set of GPU tests but rather to update this repo to have the standard tools the other repos have so GPU doesn't regress again/more.
5e56d15
to
19e8423
Compare
19e8423
to
da2efb7
Compare
Co-authored-by: David Widmann <[email protected]>
fixes #759
ForwardDiff.gradient
now supports GPU Arrayscc @ChrisRackauckas @devmotion